Skip to content

feat(autosubmit): Check diff for exactly one added block of lines - #745

Open
perlpunk wants to merge 1 commit into
os-autoinst:masterfrom
perlpunk:diff-check
Open

perlpunk wants to merge 1 commit into
os-autoinst:masterfrom
perlpunk:diff-check

Conversation

@perlpunk

@perlpunk perlpunk commented Oct 6, 2026

Copy link
Copy Markdown
Member

@perlpunk
perlpunk force-pushed the diff-check branch 3 times, most recently from 47b0959 to 35e7bf2 Compare October 6, 2026 19:03
Comment thread os-autoinst-obs-auto-submit Outdated
item.unlink()


def diff_has_single_added_block(diff_cmd: list[str]) -> bool:

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
def diff_has_single_added_block(diff_cmd: list[str]) -> bool:
def diff_has_single_added_block(diff_cmd: list[str]) -> tuple[bool, str]:

Comment on lines +277 to +290
# Ignore diff header lines (e.g., '+++ os-autoinst.changes...')
if line.startswith("+++") or line.startswith("---"):
continue

if line.startswith("-"):
return (False, None)

if line.startswith("+"):
if not in_added_block:
in_added_block = True
block_count += 1
else:
in_added_block = False

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this should be equivalent

Suggested change
# Ignore diff header lines (e.g., '+++ os-autoinst.changes...')
if line.startswith("+++") or line.startswith("---"):
continue
if line.startswith("-"):
return (False, None)
if line.startswith("+"):
if not in_added_block:
in_added_block = True
block_count += 1
else:
in_added_block = False
lines = [l for l in res.stdout.splitlines() if not l.startswith(("+++", "---"))]
if any(l.startswith("-") for l in lines):
return (False, res.stdout)
blocks = sum(1 for is_add, _ in groupby(lines, key=lambda l: l.startswith("+")) if is_add)

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I find some of the python stuff very verbose, but in this case I find my version easier to understand

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

fine, you can come up with a different way but needing to read a for loop and how a counter variable is tracked just to understand that what you want is a sum can be improved.

Drop the header lines by skipping everything before the first @@. Then use groupby on the first character of each line, so there are no counters and no header filtering:

from itertools import dropwhile, groupby

def diff_has_single_added_block(diff_cmd: list[str]) -> tuple[bool, subprocess.CompletedProcess[str]]:
    """Return True if the diff only adds one contiguous block of lines."""
    res = _run_subprocess(diff_cmd)
    hunk_lines = dropwhile(lambda line: not line.startswith("@@"), res.stdout.splitlines())
    runs = [kind for kind, _ in groupby(line[:1] for line in hunk_lines)]
    return ("-" not in runs and runs.count("+") == 1, res)

Returning the CompletedProcess keeps the callers' res.stdout working and removes the None path

Also uses dropwhile which I find quite descriptive about what it does. Sure, the first time you see it, you might be surprised but the intention should be very clear just from the function name. Also here reading a for loop and understanding the effect of "continue" might use simple control statements but one still needs to read the whole flow to understand what is going on. dropwhile and groupby in their lines when used make it clear how those lines are processed in the consecutive statements.

@okurz okurz left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

looks promising

@perlpunk
perlpunk force-pushed the diff-check branch 2 times, most recently from 3d3fd12 to 5d3b5ab Compare October 9, 2026 16:16
@perlpunk
perlpunk marked this pull request as ready for review October 9, 2026 19:53
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants